Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reorganize development docs #1200

Merged
merged 5 commits into from
Jan 24, 2023

Conversation

dai-chen
Copy link
Collaborator

@dai-chen dai-chen commented Dec 20, 2022

Signed-off-by: Chen Dai [email protected]

Description

This PR is the first one aimed to improve our dev documentation. Currently there are only a few docs in docs/dev/ folder. Actually there are many valuable info in our old docs or PR description. As initial effort, this PR added an index page reorganize existing docs to categories. Meanwhile I extracted a few PR description for main feature to new docs. Will upload more docs in separate PR.

Please see https://github.com/dai-chen/sql-1/blob/refactor-dev-docs-rebased/docs/dev/index.md -> https://github.com/opensearch-project/sql/blob/main/docs/dev/index.md

Issues Resolved

#1219

Check List

  • New functionality includes testing.
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented.
    • New functionality has javadoc added
    • New functionality has user manual doc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@dai-chen dai-chen added the documentation Improvements or additions to documentation label Dec 20, 2022
@dai-chen dai-chen self-assigned this Dec 20, 2022
@codecov-commenter
Copy link

codecov-commenter commented Dec 23, 2022

Codecov Report

Merging #1200 (c7e306f) into main (1b58f7d) will increase coverage by 0.03%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##               main    #1200      +/-   ##
============================================
+ Coverage     98.32%   98.35%   +0.03%     
- Complexity     3550     3609      +59     
============================================
  Files           344      344              
  Lines          8762     8946     +184     
  Branches        554      569      +15     
============================================
+ Hits           8615     8799     +184     
  Misses          142      142              
  Partials          5        5              
Flag Coverage Δ
sql-engine 98.35% <ø> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...c/main/java/org/opensearch/sql/expression/DSL.java 100.00% <0.00%> (ø)
...n/java/org/opensearch/sql/utils/DateTimeUtils.java 100.00% <0.00%> (ø)
...a/org/opensearch/sql/data/model/ExprDateValue.java 100.00% <0.00%> (ø)
...a/org/opensearch/sql/data/model/ExprTimeValue.java 100.00% <0.00%> (ø)
.../org/opensearch/sql/data/model/ExprValueUtils.java 100.00% <0.00%> (ø)
...g/opensearch/sql/data/model/ExprDatetimeValue.java 100.00% <0.00%> (ø)
...pensearch/sql/sql/parser/AstExpressionBuilder.java 100.00% <0.00%> (ø)
...pensearch/sql/expression/function/FunctionDSL.java 100.00% <0.00%> (ø)
...arch/sql/expression/datetime/DateTimeFunction.java 100.00% <0.00%> (ø)
...h/sql/expression/function/BuiltinFunctionName.java 100.00% <0.00%> (ø)
... and 2 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@dai-chen dai-chen changed the title Reorganize development documentation Add table of content page and reorganize dev docs Jan 3, 2023
@dai-chen dai-chen changed the title Add table of content page and reorganize dev docs Add index page and reorganize dev docs Jan 13, 2023
Signed-off-by: Chen Dai <[email protected]>
@dai-chen dai-chen marked this pull request as ready for review January 13, 2023 23:47
@dai-chen dai-chen requested a review from a team as a code owner January 13, 2023 23:47
@dai-chen dai-chen changed the title Add index page and reorganize dev docs Reorganize development docs Jan 14, 2023
@dai-chen dai-chen merged commit 1ed9701 into opensearch-project:main Jan 24, 2023
@dai-chen dai-chen deleted the refactor-dev-docs-rebased branch January 24, 2023 17:38
Copy link
Member

@YANG-DB YANG-DB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an excellent direction +1

+ **CLI**
+ **JDBC Driver**
+ **ODBC Driver**
+ **Query Workbench**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn’t this a UI feature ?
can we also have a UI section associated to the SQL ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I was planning to add some brief intro or link to their repo. Added here just to give users a complete picture of our eco system.


# OpenSearch SQL/PPL Engine Development Manual

## Introduction
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we also have tutorial section for playground like exploring

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, I'm still thinking what's the best way to do this. I have a sub task here for adding tutorial / quick start section: #1219.

Many other projects provide sample data and executable script/jar. User can play around with sample data files via CLI and their file connector. Because our CLI is written in Python, probably we can put some quick start example for playground website.

@dai-chen
Copy link
Collaborator Author

This is an excellent direction +1

@YANG-DB I've merged this PR right before your comments. Feel free to comment on #1219 if any idea. I will do it in the future PRs. Thanks!

* *Observability :* The ability to understand whats happening inside your business/application using logs, traces, metrics and other data emitted from the application.


## 3.Tenets
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
## 3.Tenets
## 3. Tenets

* Changes to PPL Query Language grammar should be simple and easy to onboard.
* Component design should be extensible for supporting new data stores.

## 4.Out of Scope for the Design.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
## 4.Out of Scope for the Design.
## 4. Out of Scope for the Design

Comment on lines +47 to +49



Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change




### 6.1 Data source representation.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
### 6.1 Data source representation.
### 6.1 Data source representation

* *Catalog :* we can describe a catalog as the union of a connector and the underlying instance of the datasource being referred. This gives us flexbility to refer different instances of a similar datastore with the same connector i.e. we can refer data from multiple prometheus instances using prometheus connector. The name for each catalog should be different.

Example Prometheus Catalog Definition
```
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
```
```json

Comment on lines +109 to +110

* JSON Format
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* JSON Format
  • JSON Format

Comment on lines +124 to +128
[
{"age": ExprIntegerValue(1), "account_number": ExprIntegerValue(1)},
{"age": ExprNullValue(), "account_number": ExprIntegerValue(2)},
{"account_number": ExprIntegerValue(3)}
]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[
{"age": ExprIntegerValue(1), "account_number": ExprIntegerValue(1)},
{"age": ExprNullValue(), "account_number": ExprIntegerValue(2)},
{"account_number": ExprIntegerValue(3)}
]
```json
[
{"age": ExprIntegerValue(1), "account_number": ExprIntegerValue(1)},
{"age": ExprNullValue(), "account_number": ExprIntegerValue(2)},
{"account_number": ExprIntegerValue(3)}
]

``

##### Include NULL and MISSING value in the QueryResult (Issue 1, 2)
The SELECT operator will be translated to PhysicalOpeartor with a list of expression to resolve ExprValue from input data. With the above example, when handling NULL and MISSING value, the expected output data should be as follows.

```
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
```
```json


An additional list of Schema is also required to when protocol is JDBC.

```
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
```
```json

1. Each data source adds special logical operator and explode the optimizer rule space. For example, Prometheus also has `PrometheusLogicalMetricAgg` and `PrometheusLogicalMetricScan` accordingly. They have the exactly same pattern to match query plan tree as OpenSearch.
2. A bigger problem is the difficulty of transforming from logical to physical when there are 2 `Table`s in query plan. Because only 1 of them has the chance to do the `implement()`. This is a blocker for supporting `INSERT ... SELECT ...` statement or JOIN query. See code below.

```
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
```
```java

@dai-chen
Copy link
Collaborator Author

This is not back port to other branch intentionally. Because I think we may need single dedicated branch for Github pages in future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
Status: Planned work items
Development

Successfully merging this pull request may close these issues.

6 participants